🔧 from_toml() does not read environment variables — there's even a test proving this (test_from_toml_ignores_env_vars). The original build.rs used from_toml_and_env() to merge TRUSTED_SERVER__* env vars into the binary config. This silently breaks deployments that set secrets (API keys, origin URLs) via env vars at build time.

let mut settings = settings::Settings::from_toml_and_env(&toml_content)
    .expect("Failed to parse settings at build time");

🔧 Settings::default() returns an empty IntegrationSettings HashMap, so collect_env_vars will never discover any TRUSTED_SERVER__INTEGRATIONS__* env vars. Changing e.g. TRUSTED_SERVER__INTEGRATIONS__DATADOME__SERVER_SIDE_KEY won't trigger a rebuild.

The old build.rs used an always-rebuild sentinel (_always_rebuild_sentinel_) specifically because integration keys can't be enumerated. Consider restoring that approach.

🔧 This sums ASCII byte values — IPs like "1.2.3.4" and "4.3.2.1" produce identical hashes. IPs in the same /24 subnet cluster together since only the last octet differs. Sampling will be very non-uniform.

let hash = client_ip.bytes().fold(0u32, |acc, b| {
    acc.wrapping_mul(31).wrapping_add(u32::from(b))
});

🔧 This field is validated (50-1000ms range) but never applied — the comment at line 564 says Fastly doesn't support per-request timeouts. Users setting validation_timeout_ms = 100 would expect it to take effect. Either wire it into BackendConfig timeout support or remove the field to avoid misleading configuration.

❓ This runs on every proxied request — CSS, JS, images, fonts all trigger a DataDome HTTP call (200ms timeout). Most bot protection validates only document/navigation requests. Is this intentional? If so, worth a comment explaining why.

🤔 Invalid regex in config will expect() panic on the first matching request, not at startup. BackendRouter::new() doc says it returns errors for invalid patterns but doesn't actually validate them. Consider compiling the regex eagerly in the constructor and returning the error there.

🤔 If two backends claim the same domain (or domains that normalize to the same value), the second silently overwrites the first. Could cause hard-to-debug routing. Consider detecting duplicates and logging a warning.

♻️ ~30 lines of code for "unused - for future use". Consider removing until actually wired into BackendConfig.

⛏ Allocates twice on every request (to_lowercase() then to_string() after trimming). Domains stored in the index are already normalized at construction time, but the incoming host is re-normalized on every call. Could use Cow<str> to avoid allocation when no transformation is needed.

⛏ If the field is unused, consider removing it rather than suppressing the warning.

🔧 P0 — Env var overrides no longer applied at build time

This changed from from_toml_and_env → from_toml, which means environment variables like TRUSTED_SERVER__INTEGRATIONS__DATADOME__SERVER_SIDE_KEY are no longer merged at build time. The rerun_if_changed function below still enumerates env vars, but they're never applied — making it dead code.

Fix: Restore from_toml_and_env, or if this is intentional, remove the rerun_if_changed env var enumeration and document why env vars are no longer baked in.

🔧 P1 — Sampling hash has poor distribution for IPv4

Simple byte-sum produces identical hashes for IPs with the same octets in different order (1.2.3.4 == 4.3.2.1). With % 100, sampling will be far from uniform.

Fix:

use std::hash::{Hash, Hasher};
let mut hasher = std::collections::hash_map::DefaultHasher::new();
client_ip.hash(&mut hasher);
let sample = (hasher.finish() % 100) as u8;

🔧 P1 — Validation runs on every request including static assets

validate_request_server_side is called unconditionally — images, CSS, JS, fonts all trigger a blocking HTTP call to DataDome (200ms default timeout). This:

• Adds latency to every static asset
• Multiplies DataDome API usage/cost
• Could hit DataDome rate limits

DataDome validation is typically needed only for document/HTML requests.

Fix: Filter by request path or accept header to skip known static assets, or at minimum only validate navigational requests.

🔧 P1 — Duplicate domain silently overwrites

If two backends declare the same domain, HashMap::insert silently keeps the last one. This can cause hard-to-debug routing issues in production.

Fix: Log a warning or return an error:

if let Some(existing) = domain_index.insert(normalized.clone(), idx) {
    log::warn!("Domain '{}' appears in multiple backends (index {} and {})", normalized, existing, idx);
}

🤔 P2 — validation_timeout_ms has no effect

This field is validated and documented, but the comment at line 564 says Fastly Compute doesn't support per-request timeouts. Operators will configure this expecting it to work.

Fix: Either remove the field, or log a warning at startup when it's set to a non-default value so operators aren't misled.

🤔 P2 — BackendTimeouts unused

The comment says "unused - for future use." This adds config surface area and serialization overhead with no current benefit. Per project conventions: don't design for hypothetical future requirements.

🤔 P2 — Inconsistent cookie trimming

trim_start() here vs trim() in datadome.rs:521 for the same cookie-parsing pattern. Standardize on trim() for robustness.

🤔 P2 — No test coverage for validate_request logic

The entire server-side validation flow — sampling, cookie extraction, request building, response status handling, fail-open/closed — has no unit tests. The #[cfg(test)] in get_client_ip shows testing was considered but the actual validation path is uncovered.